fix(standards): report every storage size a standard note accepts - #3810
Merged
Conversation
zeapoz
reviewed
Sep 7, 2026
| /// `SetMaxSupply` uses this size; no size between it and [`Self::MAX_NUM_STORAGE_ITEMS`] | ||
| /// is valid. Keep in sync with `NUM_ITEMS_SET_MAX_SUPPLY` in | ||
| /// `faucet_metadata_config.masm`. | ||
| pub const MIN_NUM_STORAGE_ITEMS: usize = 2; |
Collaborator
There was a problem hiding this comment.
Doesn't necessarily have to be in this PR, but maybe it makes sense to start representing the number of storage items with a simple enum as we have very distinct variants now? Something like:
enum NumStorageItems {
// An exact count of storage items.
Exact(usize),
// A range of acceptable storage item counts.
Range { min: usize, max: usize },
}
mmagician
approved these changes
Sep 8, 2026
zeapoz
reviewed
Sep 8, 2026
zeapoz
left a comment
zeapoz
left a comment
Collaborator
There was a problem hiding this comment.
Looks pretty good! I think it would be nice if we could make the Rust NUM_STORAGE_ITEMS be a NumStorageItems enum to enable some cleaner logic.
Comment on lines
+190
to
+197
| /// Returns the number of storage items this kind of note accepts. | ||
| /// | ||
| /// Several note kinds accept more than one storage size, so no single expected size can be | ||
| /// derived from the script root alone: a MINT note holds exactly | ||
| /// [`MintNote::NUM_STORAGE_ITEMS_PRIVATE`] items when it creates a private output note and at | ||
| /// least [`MintNote::MIN_NUM_STORAGE_ITEMS_PUBLIC`] when it creates a public one, and the | ||
| /// config notes size their storage per action. The returned value mirrors the sizes each note | ||
| /// script accepts; use [`NumStorageItems::accepts`] to check one against it. |
Collaborator
There was a problem hiding this comment.
Suggested change
| /// Returns the number of storage items this kind of note accepts. | |
| /// | |
| /// Several note kinds accept more than one storage size, so no single expected size can be | |
| /// derived from the script root alone: a MINT note holds exactly | |
| /// [`MintNote::NUM_STORAGE_ITEMS_PRIVATE`] items when it creates a private output note and at | |
| /// least [`MintNote::MIN_NUM_STORAGE_ITEMS_PUBLIC`] when it creates a public one, and the | |
| /// config notes size their storage per action. The returned value mirrors the sizes each note | |
| /// script accepts; use [`NumStorageItems::accepts`] to check one against it. | |
| /// Returns the [`NumStorageItems`] items this kind of note accepts. |
| Range { min: usize, max: usize }, | ||
| /// The note holds a number of storage items accepted by any of these, and by none of the | ||
| /// sizes in between them. | ||
| AnyOf(&'static [NumStorageItems]), |
| /// `AcceptOwnership` / `RenounceOwnership` use this size; no size between it and | ||
| /// [`Self::MAX_NUM_STORAGE_ITEMS`] is valid. Keep in sync with `NUM_ITEMS_*` in | ||
| /// `owner_config.masm`. | ||
| pub const MIN_NUM_STORAGE_ITEMS: usize = 1; |
Collaborator
There was a problem hiding this comment.
Question: do we have to have these usize constants, I know that we have to sync these between MASM and Rust, but it would be nice if we could directly define e.g.:
pub const NUM_STORAGE_ITEMS: NumStorageItems = NumStorageItems::Range { min: 1, max: 3 };I think any reasonable LLM should be able to make the link between the named MIN/MAX constants on the MASM side and this typed enum.
| Self::NETWORK_ACCOUNT_CONFIG => NetworkAccountConfigNote::NUM_STORAGE_ITEMS, | ||
| Self::FEE_SPONSORSHIP => FeeSponsorshipNote::NUM_STORAGE_ITEMS, | ||
| Self::TX_FEE => TxFeeNote::NUM_STORAGE_ITEMS, | ||
| Self::P2ID => NumStorageItems::Exact(P2idNote::NUM_STORAGE_ITEMS), |
Collaborator
There was a problem hiding this comment.
// An exact count.
Self::P2ID => P2idNote::NUM_STORAGE_ITEMS,
// Any of relation with an exact count and ranged count.
Self::MINT => NumStorageItems::AnyOf(&[
MintNote::NUM_STORAGE_ITEMS_PRIVATE,
MintNote::NUM_STORAGE_ITEMS_PUBLIC,
]),If we change the NUM_STORAGE_ITEMS constant to be a NumStorageItems, then this dispatch could be a lot cleaner, see my other comment.
partylikeits1983
approved these changes
Sep 8, 2026
partylikeits1983
left a comment
partylikeits1983
left a comment
Contributor
There was a problem hiding this comment.
Looks great!
Only thing that needs fixing I think is the changelog.
mmagician
approved these changes
Sep 8, 2026
mmagician
pushed a commit
that referenced
this pull request
Sep 9, 2026
Base update, bringing in the v0.17.0-rc.4 version bump (#3837). No conflicts, but the CHANGELOG needed a manual pass anyway: #3810's entry auto-merged under the heading this branch renamed to v0.17.0-rc.3, and it is not in that release. Moved to rc.4. The rc.3 section again carries exactly the published entry set. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_015KNLFFu1vkaRFyybnKAYfw
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
StandardNote::expected_num_storage_itemswithaccepts_num_storage_items, which accepts13items for a privateMINTnote and20or more for a public one.1or3forOWNER_CONFIG,2to4forRBAC_CONFIG,2or32forFAUCET_METADATA_CONFIG.MIN_NUM_STORAGE_ITEMStoOwnerConfigNote,RbacConfigNoteandFaucetMetadataConfigNote.MINTstorage item counts to the constants inasm/standards/notes/mint/.Closes #3809.